-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Panda + Tiger + Eagle #7
base: master
Are you sure you want to change the base?
Conversation
@@ -3,22 +3,47 @@ | |||
require "ostruct" | |||
require_relative "./movie" | |||
class Api | |||
@@search_history = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like class variables in most cases... This will prevent you from going multi-threaded in the future (as each thread will overwrite this variable)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that is a left over artifact from before I implemented the Search_History class.
Good stuff here! I especially love the way the search_history class ended up looking... Looks great (small methods, readable, clean). One minor note --- I found that if I had some valid searches, and then mispelled a classic (Billy Maddison), it would use 0 as a score and drop my movie score
|
Thank you for your great suggestions. I will add a test for the NOTHINGFOUNDHERE score bug. |
I still need to refactor my test, but i hope to go over that in the office hour if you don't already have a plan.